fix this.import from node_modules in v1 addons #1488
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
void-mAlex
approved these changes
Jun 29, 2023
|
Merged, but a thought for next time: it would be good to consolidate scenarios better. Not every test needs a whole app around it and the test suite overhead is considerable. We could make it clearer that there's a standard scenario for testing app features and a standard scenario for testing addon features and a test like this could be one new test in the addon scenario. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of #1435 we have moved all vendor code from addons to
./node_modules/.embroider/rewritten-packages/@embroider/synthesized-vendor/, which works when you callthis.import()from an addon'sincluded()hook.Perhaps as a copy & paste error we were ignoring the case where
this.import()was called on a file innode_modules, this PR will fix that.Note: I have committed and pushed a failing test that I will fix in a subsequent commit so that we can see that the issue exists on main and is fixed by the commit 👍